Auto-recover from corrupt Wasm builds that emit undefined env imports#4287
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces automatic recovery for corrupt incremental WebAssembly builds. It adds a new sequence_then utility to run follow-up steps after a command sequence completes, and uses it to detect corrupt env imports in the generated JS glue, wiping the target directory and rebuilding if necessary. The review feedback correctly identifies a potential race condition in sequence_then where the follow_up closure (which deletes the target directory) could still execute after the sequence is killed, and suggests checking the kill flag before proceeding.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
a906395 to
5e832b7
Compare
|
@Keavon I have started the AI code review. It will take a few minutes to complete. |
env importsenv imports
There was a problem hiding this comment.
2 issues found across 3 files
Confidence score: 2/5
- In
tools/cargo-run/src/frontend.rs, the recovery logic needs to recognize the single-quotedenvimport pattern emitted by wasm-bindgen; if it doesn’t, poisoned glue can slip past detection and the self-heal path won’t run. Add this import variant to the detector before merging so recovery is consistently triggered. - In
tools/cargo-run/src/cmd.rs,follow_upcan still run after a watcher build is cancelled unless the killed flag is checked first, which risks cleanup side effects and then skipping the recovery rebuild. Guardfollow_upbehind an early killed check before merging to avoid wiping/racing build artifacts.
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
5a2c638 to
b9caf08
Compare
There was a problem hiding this comment.
3 issues found across 3 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="tools/cargo-run/src/frontend.rs">
<violation number="1" location="tools/cargo-run/src/frontend.rs:91">
P3: Rename `is_build_corupted` to `is_build_corrupted` to avoid carrying a misspelled public helper name.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
timon-schelling
left a comment
There was a problem hiding this comment.
with my changes applied LGTM
- cmd.rs: skip the follow-up when the sequence was killed, so a build the watcher superseded doesn't run the destructive heal and force the next build into an unnecessary full rebuild. - frontend.rs: also match a single-quoted `from 'env'` marker so a wasm-bindgen quote-style change can't hide poisoned glue from the detector.
11f064b to
6bc0efc
Compare
An interrupted or concurrent
cargo build --target wasm32-unknown-unknowncan leave a wasm crate's incremental codegen inconsistent, producing a.wasmwith undefined symbols that rust-lld emits asenvimports. wasm-bindgen writes these into the JS glue asimport … from "env", which Vite can't resolve, so the build succeeds but the dev server breaks, previously needing a manualcargo clean.cargo-run now checks the generated glue for the
from "env"marker after each wasm build and, when present, wipes only thewasm32-unknown-unknowntarget directory (not the shared hostdebug/buildartifacts, which other tools can hold locked on Windows) and rebuilds once. This runs on both the startup build and the watcher's rebuilds via a newsequence_then, which appends the recovery steps to the same killable sequence.Example of a poisoned scenario's terminal output after opening the localhost page in a browser: